Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a layers panel #17

Merged
merged 11 commits into from
Jun 27, 2024
Merged

Add a layers panel #17

merged 11 commits into from
Jun 27, 2024

Conversation

brichet
Copy link
Collaborator

@brichet brichet commented Jun 24, 2024

Add a layer panel to the left.

Peek 2024-06-26 12-01

Remaining for this PR:

  • Add button to toggle visible
  • Add icons for the layer type
  • Add tests

Probably in a follow up PR:

  • display the maps in the layer tree order (and not the layers list order)

Copy link
Contributor

Binder 👈 Launch a Binder on branch brichet/jupytergis/layers

@brichet brichet changed the title Layers Add a layers panel Jun 24, 2024
@brichet brichet added the enhancement New feature or request label Jun 24, 2024
packages/schema/src/doc.ts Outdated Show resolved Hide resolved
packages/schema/src/doc.ts Outdated Show resolved Hide resolved
packages/schema/src/doc.ts Outdated Show resolved Hide resolved
packages/schema/src/doc.ts Outdated Show resolved Hide resolved
@brichet brichet marked this pull request as ready for review June 26, 2024 09:59
@brichet brichet requested a review from martinRenou June 26, 2024 11:05
Copy link
Member

@martinRenou martinRenou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing! Thanks!

My comments are only nitpicky comments about the naming of layersTree. It seems a bit heterogeneous in the code-base between layersTree, layerTree and treeLayers so I'm suggesting going for layersTree everywhere.

packages/schema/src/schema/jgis.json Outdated Show resolved Hide resolved
packages/schema/src/doc.ts Outdated Show resolved Hide resolved
@brichet
Copy link
Collaborator Author

brichet commented Jun 26, 2024

My comments are only nitpicky comments about the naming of layersTree. It seems a bit heterogeneous in the code-base between layersTree, layerTree and treeLayers so I'm suggesting going for layersTree everywhere.

Your are right for some of them, but maybe not for all.
I wanted to make the distinction between the tree itself (a IJGISLayerGroup object, what I called layerTree), and the list of layers coming from the tree (a (IJGISLayerGroup | string)[] object, that I called treeLayers).

The second come from the fact that I used a IJGISLayerGroup for the tree but we only need the list of layers, the other properties are useless. See #4 (comment) for reference.

Maybe we should declare the tree as a (IJGISLayerGroup | string)[] to simplify.
The only drawback is the duplication of the following schema, for the layersTree itself and the layers of a IJGISLayerGroup.

"items": {
  "oneOf": [
      {"type": "string"},
      {
        "type": "object",
        "$ref": "#/definitions/jGISLayerGroup"
      }
  ]
}

It would clarify the naming and the tree object, avoiding useless properties, since the tree should only be a list of layers and groups (if I'm not mistaken).

@martinRenou
Copy link
Member

Oh ok I see.

Maybe we should declare the tree as a (IJGISLayerGroup | string)[] to simplify

That sounds good to me. Then you only operate on this layersTree (the root) which is of this type.

@brichet
Copy link
Collaborator Author

brichet commented Jun 26, 2024

Oh ok I see.

Maybe we should declare the tree as a (IJGISLayerGroup | string)[] to simplify

That sounds good to me. Then you only operate on this layersTree (the root) which is of this type.

Exact, let's update it then.

@brichet brichet marked this pull request as draft June 26, 2024 15:32
@brichet brichet marked this pull request as ready for review June 27, 2024 07:30
@brichet
Copy link
Collaborator Author

brichet commented Jun 27, 2024

@martinRenou I updated the schema

Copy link
Member

@martinRenou martinRenou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great thanks!

@martinRenou martinRenou merged commit fd044fb into geojupyter:main Jun 27, 2024
9 checks passed
@brichet brichet deleted the layers branch June 27, 2024 08:00
gjmooney pushed a commit to martinRenou/jupytergis that referenced this pull request Jun 27, 2024
* Add the layer tree in shared document

* Very first version of layers panel

* Add a layers list under group, and a current selected layer in IJupyterGISModel

* Add forgotten file from previous commit

* Fix doc, lint and add docstring

* Add an icon for raster layers

* Style on icon

* Add a visibility icon and fix the visibility layer with mapLibre

* Add ui tests

* Updates the example jGIS file after rebase

* Chnage the LayersTree to a list of Layer or LayersGroup, and harmonize names
@brichet brichet mentioned this pull request Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants